feat: integrate feeabs #175
Conversation
WalkthroughThe changes implemented introduce fee abstraction into the Eve chain, ensuring its compatibility with sdk 50 and integrating fee abstraction (FA) functionalities. This includes the addition of new imports, keepers, and configurations necessary for FA, aiming to make Eve the first live chain with fee abstraction. Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.modis excluded by:!**/*.modgo.sumis excluded by:!**/*.sum
Files selected for processing (2)
- app/ante.go (4 hunks)
- app/app.go (17 hunks)
Additional comments: 11
app/ante.go (3)
- 32-32: The addition of
FeeAbskeeperto theHandlerOptionsstruct is a crucial part of integrating fee abstraction. However, ensure that all instances whereHandlerOptionsis instantiated throughout the codebase are updated to include this new field to avoid runtime errors.Verification successful
To thoroughly verify the integration of
FeeAbskeeperin all instances ofHandlerOptions, it's necessary to search the entire codebase for any other instantiations. This will help confirm whether the absence ofFeeAbskeeperin the provided instance is an isolated case or a widespread oversight.
To accurately search for all instances of
HandlerOptionsinstantiation across the codebase, it's necessary to correct the regex syntax error in the previous script and re-execute the search.
The verification process has confirmed that the
FeeAbskeeperfield is not included in any of the instances whereHandlerOptionsis instantiated, as shown in the provided script output. This aligns with the initial review comment, emphasizing the need to update all instances ofHandlerOptionsinstantiation to include theFeeAbskeeperfield to avoid runtime errors.* 63-63: The use of `NewFeeAbstrationMempoolFeeDecorator` from `feeabsante` is a significant addition. However, there's a typographical error in the function name (`Abstration` should be `Abstraction`). This might be a typo or an inconsistency in the library's API. Please verify the correct function name in the `feeabsante` package. * 69-69: The integration of `NewFeeAbstractionDeductFeeDecorate` is key to the fee abstraction functionality. However, ensure that the naming is consistent with the library's API. If `Decorate` is intended, it's fine; otherwise, it might be a typo for `Decorator`. Verify the correct function name in the `feeabsante` package.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for instantiation of HandlerOptions to ensure FeeAbskeeper is included. ast-grep --lang go --pattern $'HandlerOptions{$$$}'Length of output: 842
Script:
#!/bin/bash # Search for all instantiations of HandlerOptions across the entire codebase. rg "HandlerOptions{" -A 10Length of output: 122
Script:
#!/bin/bash # Corrected search for all instantiations of HandlerOptions across the entire codebase. rg "HandlerOptions\{" -A 10Length of output: 976
app/app.go (8)
- 209-214: The addition of
feeabstypes.ModuleNametomaccPermsandallowedReceivingModAccmaps is a crucial part of integrating fee abstraction functionalities. This ensures that the fee abstraction module has the necessary permissions and can receive tokens. This change aligns with the PR's objective to enhance fee management within the application.- 254-254: The
FeeabsKeeperis correctly added to theEveAppstruct, enabling the application to manage fee abstraction functionalities effectively. This addition is essential for the integration of fee abstraction into the application's architecture.- 274-274: The creation of a scoped fee abstraction keeper (
ScopedFeeabsKeeper) is a critical step in setting up the fee abstraction functionalities. This scoped keeper ensures that capabilities are correctly managed and isolated for the fee abstraction module, aligning with best practices for capability management in Cosmos SDK applications.- 582-584: The handling of fee abstraction proposals within the governance router is correctly implemented. This allows for the governance mechanism to interact with fee abstraction functionalities, such as updating host zone configurations, which is crucial for the flexible management of fee abstraction settings.
- 656-667: The initialization and setup of the
FeeabsKeeperare correctly implemented. This setup includes configuring the keeper with necessary dependencies such as theStakingKeeper,AccountKeeper,BankKeeper, and scoped keeper for fee abstraction. This comprehensive setup is essential for the fee abstraction functionalities to operate effectively within the application.- 702-703: The integration of the fee abstraction module into the IBC router is correctly implemented. This allows for the fee abstraction functionalities to interact with IBC, enabling the application to manage fees for inter-blockchain communication effectively. This integration is a key part of extending the fee abstraction capabilities to a broader range of operations within the application.
- 832-832: The registration of the fee abstraction module in the application's module manager is correctly implemented. This ensures that the fee abstraction functionalities are included in the application's lifecycle, including initialization, begin block, end block, and genesis setup. This registration is crucial for the seamless integration of fee abstraction into the application.
- 846-848: The registration of fee abstraction proposal handlers in the governance module's basic manager is correctly implemented. This allows for the governance mechanism to process proposals related to fee abstraction, such as adding, deleting, or setting host zones. This registration is essential for enabling governance control over fee abstraction settings.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.modis excluded by:!**/*.modgo.sumis excluded by:!**/*.sum
Files selected for processing (2)
- app/ante.go (3 hunks)
- app/app.go (16 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/app.go
Additional comments: 1
app/ante.go (1)
- 31-31: The addition of the
FeeAbskeeperfield to theHandlerOptionsstruct is a necessary change to support fee abstraction functionalities. The naming and type are correctly specified.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.modis excluded by:!**/*.modgo.sumis excluded by:!**/*.sum
Files selected for processing (1)
- app/ante.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/ante.go
|
@coderabbitai - fee abstraction allows eve to accept fees in the ibc denom of any of its ibc counteryparty chains. This makes her easier to use. This process is secure because the foriegn denoms are swapped back to $eve on osmosis. |
|
Tip For best results, initiate chat on the files or code changes. @faddat, thank you for sharing how fee abstraction enhances the usability and security of the eve application by allowing it to accept fees in the IBC denom of any of its IBC counterpart chains. The process of swapping foreign denoms back to $eve on Osmosis indeed adds a layer of security and flexibility, making the application more accessible and user-friendly. This integration is a significant step towards improving the fee management framework within the application. |
|
It seems feeabs v8 also has pfm v8 as dependency, so need to take that into account. |
There was a problem hiding this comment.
Actionable comments posted: 5
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum
Files selected for processing (2)
- app/ante.go (3 hunks)
- app/app.go (16 hunks)
Additional Context Used
GitHub Check Runs (3)
lint failure (8)
app/ante.go: [failure] 1-1:
: # github.com/eve-network/eve/app
app/app.go: [failure] 622-622:
cannot use app.BankKeeper (variable of type "github.com/terra-money/alliance/custom/bank/keeper".Keeper) as "github.com/cosmos/cosmos-sdk/x/bank/keeper".BaseKeeper value in argument to feeabskeeper.NewKeeper
app/app.go: [failure] 662-662:
undefined: AddRoute (typecheck)
app/app.go: [failure] 622-622:
cannot use app.BankKeeper (variable of type "github.com/terra-money/alliance/custom/bank/keeper".Keeper) as "github.com/cosmos/cosmos-sdk/x/bank/keeper".BaseKeeper value in argument to feeabskeeper.NewKeeper
app/app.go: [failure] 662-662:
undefined: AddRoute) (typecheck)build failure (3)
app/app.go: [failure] 622-622:
cannot use app.BankKeeper (variable of type "github.com/terra-money/alliance/custom/bank/keeper".Keeper) as "github.com/cosmos/cosmos-sdk/x/bank/keeper".BaseKeeper value in argument to feeabskeeper.NewKeeper
app/app.go: [failure] 662-662:
undefined: AddRouterun-tests failure (3)
app/app.go: [failure] 622-622:
cannot use app.BankKeeper (variable of type "github.com/terra-money/alliance/custom/bank/keeper".Keeper) as "github.com/cosmos/cosmos-sdk/x/bank/keeper".BaseKeeper value in argument to feeabskeeper.NewKeeper
app/app.go: [failure] 662-662:
undefined: AddRoute
Additional comments not posted (8)
app/ante.go (2)
32-32: The addition ofFeeAbskeeperto theHandlerOptionsstruct is a necessary change to support fee abstraction. This change is well-integrated and follows the existing structure of the file.
69-69: The functionNewFeeAbstractionDeductFeeDecorateseems to be incorrectly named or possibly a typo. It's more conventional to name such functions ending with "Decorator" to align with the naming convention of other decorators. Please verify if this is a typo or if the function name needs to be corrected.Verification successful
The function
NewFeeAbstractionDeductFeeDecoratehas been verified to exist in the codebase with the specified name. The concern regarding the naming convention (whether it should end with "Decorator" instead of "Decorate") is subjective and should be evaluated in the context of the project's standards for naming conventions. Therefore, the review comment is based on a valid observation, but any decision on renaming should align with the project's naming conventions.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if the function name is a typo or needs correction ast-grep --lang go --pattern $'NewFeeAbstractionDeductFeeDecorate($_, $_, $_, $_)'Length of output: 83
Script:
#!/bin/bash # Search for functions with a similar name to verify the correct naming convention rg --glob '*.go' 'NewFeeAbstractionDeductFee'Length of output: 195
app/app.go (6)
47-49: The imports forfeeabsmodule,feeabskeeper, andfeeabstypesare correctly added to integrate fee abstraction functionalities. Ensure that thego.sumfile is updated with the necessary entries for these packages to avoid potential build or module resolution issues.
208-208: The declaration offeeabstypes.ModuleNamewithin themaccPermsmap is correctly added, indicating that the fee abstraction module does not require any specific permissions for its associated module account. This is consistent with the integration of new modules.
248-248: TheFeeabsKeeperis correctly declared within theEveAppstruct, ensuring that the fee abstraction keeper is accessible throughout the application. This is a necessary step for integrating new functionalities.
267-267: The declaration ofScopedFeeabsKeeperwithin theEveAppstruct is correctly added, ensuring that the fee abstraction keeper has a scoped keeper for capability management. This is crucial for the secure and modular handling of capabilities within the application.
335-335: The addition offeeabstypes.StoreKeyto the list of KVStore keys is correctly implemented. This ensures that the fee abstraction module has its dedicated storage space within the application's state, which is necessary for storing and managing its state.
387-387: TheScopedFeeabsKeeperis correctly scoped to the fee abstraction module using theScopeToModulemethod. This is an essential step for capability management, ensuring that the fee abstraction module can securely claim capabilities.
app/app.go
Outdated
| app.FeeabsKeeper = feeabskeeper.NewKeeper( | ||
| appCodec, | ||
| app.keys[feeabstypes.StoreKey], | ||
| app.GetSubspace(feeabstypes.ModuleName), | ||
| &app.StakingKeeper, | ||
| app.AccountKeeper, | ||
| app.BankKeeper, | ||
| app.TransferKeeper, | ||
| app.IBCKeeper.ChannelKeeper, | ||
| app.IBCKeeper.PortKeeper, | ||
| app.ScopedFeeabsKeeper, | ||
| ) |
There was a problem hiding this comment.
The instantiation of the FeeabsKeeper using feeabskeeper.NewKeeper is correctly implemented, with all necessary dependencies passed in. However, there's a type mismatch issue with the BankKeeper argument.
- app.BankKeeper,
+ app.BankKeeper.BaseKeeper,Ensure to pass the BaseKeeper from the custom BankKeeper to match the expected interface.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| app.FeeabsKeeper = feeabskeeper.NewKeeper( | |
| appCodec, | |
| app.keys[feeabstypes.StoreKey], | |
| app.GetSubspace(feeabstypes.ModuleName), | |
| &app.StakingKeeper, | |
| app.AccountKeeper, | |
| app.BankKeeper, | |
| app.TransferKeeper, | |
| app.IBCKeeper.ChannelKeeper, | |
| app.IBCKeeper.PortKeeper, | |
| app.ScopedFeeabsKeeper, | |
| ) | |
| app.FeeabsKeeper = feeabskeeper.NewKeeper( | |
| appCodec, | |
| app.keys[feeabstypes.StoreKey], | |
| app.GetSubspace(feeabstypes.ModuleName), | |
| &app.StakingKeeper, | |
| app.AccountKeeper, | |
| app.BankKeeper.BaseKeeper, | |
| app.TransferKeeper, | |
| app.IBCKeeper.ChannelKeeper, | |
| app.IBCKeeper.PortKeeper, | |
| app.ScopedFeeabsKeeper, | |
| ) |
app/app.go
Outdated
| @@ -637,6 +659,7 @@ | |||
| AddRoute(wasmtypes.ModuleName, wasmStack). | |||
| AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). | |||
| AddRoute(icahosttypes.SubModuleName, icaHostStack) | |||
| AddRoute(feeabstypes.ModuleName, feeabsIBCModule) | |||
There was a problem hiding this comment.
There's an issue with the AddRoute method call for adding the fee abstraction module to the IBC router. This method call is undefined, indicating a potential syntax error or missing implementation.
Please verify the correct method to add a route for the fee abstraction module to the IBC router. It's possible that the method name or the way routes are added to the router has changed or needs to be implemented.
| feeabsante "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/ante" | ||
| feeabskeeper "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/keeper" |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process confirms that both feeabsante and feeabskeeper dependencies are missing from the go.sum file. It's recommended to run go mod tidy to update the go.sum file accordingly and avoid potential build failures.
Analysis chain
The imports for feeabsante and feeabskeeper are correctly added to integrate fee abstraction functionalities. However, ensure that the go.sum file is updated with these dependencies to avoid build failures. Running go mod tidy should address this.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if go.sum contains entries for feeabsante and feeabskeeper
if ! grep -q "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/ante" go.sum; then
echo "Missing go.sum entry for feeabsante."
fi
if ! grep -q "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/keeper" go.sum; then
echo "Missing go.sum entry for feeabskeeper."
fi
Length of output: 319
The import statements are not organized according to Go conventions, which recommend grouping and ordering imports for better readability. Consider using a tool like gci to automatically format the import statements.
import (
"errors"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/ante"
ibcante "github.com/cosmos/ibc-go/v8/modules/core/ante"
"github.com/cosmos/ibc-go/v8/modules/core/keeper"
feeabsante "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/ante"
feeabskeeper "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/keeper"
wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
wasmTypes "github.com/CosmWasm/wasmd/x/wasm/types"
corestoretypes "cosmossdk.io/core/store"
circuitante "cosmossdk.io/x/circuit/ante"
circuitkeeper "cosmossdk.io/x/circuit/keeper"
)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| feeabsante "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/ante" | |
| feeabskeeper "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/keeper" | |
| import ( | |
| "errors" | |
| sdk "github.com/cosmos/cosmos-sdk/types" | |
| "github.com/cosmos/cosmos-sdk/x/auth/ante" | |
| ibcante "github.com/cosmos/ibc-go/v8/modules/core/ante" | |
| "github.com/cosmos/ibc-go/v8/modules/core/keeper" | |
| feeabsante "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/ante" | |
| feeabskeeper "github.com/osmosis-labs/fee-abstraction/v8/x/feeabs/keeper" | |
| wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper" | |
| wasmTypes "github.com/CosmWasm/wasmd/x/wasm/types" | |
| corestoretypes "cosmossdk.io/core/store" | |
| circuitante "cosmossdk.io/x/circuit/ante" | |
| circuitkeeper "cosmossdk.io/x/circuit/keeper" | |
| ) |
| @@ -57,11 +60,13 @@ | |||
| wasmkeeper.NewGasRegisterDecorator(options.WasmKeeper.GetGasRegister()), | |||
| circuitante.NewCircuitBreakerDecorator(options.CircuitKeeper), | |||
| ante.NewExtensionOptionsDecorator(options.ExtensionOptionChecker), | |||
| feeabsante.NewFeeAbstrationMempoolFeeDecorator(options.FeeAbskeeper), | |||
There was a problem hiding this comment.
There's a typographical error in the decorator function name NewFeeAbstrationMempoolFeeDecorator. It should be corrected to NewFeeAbstractionMempoolFeeDecorator to maintain consistency and avoid potential runtime errors.
- feeabsante.NewFeeAbstrationMempoolFeeDecorator(options.FeeAbskeeper),
+ feeabsante.NewFeeAbstractionMempoolFeeDecorator(options.FeeAbskeeper),Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| feeabsante.NewFeeAbstrationMempoolFeeDecorator(options.FeeAbskeeper), | |
| feeabsante.NewFeeAbstractionMempoolFeeDecorator(options.FeeAbskeeper), |
|
@minhngoc274 please check some error |
I just updated @hoank101 , @tnv1 is there anythings else i need to implement, please review it for me |
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- app/app.go (16 hunks)
Additional comments not posted (6)
app/app.go (6)
47-49: The imports forfeeabsmodule,feeabskeeper, andfeeabstypesare correctly added to integrate the fee abstraction functionalities.
248-248: The declaration ofFeeabsKeeperandScopedFeeabsKeeperwithin theEveAppstruct is correctly done to manage state and capabilities for the fee abstraction module.Also applies to: 267-267
208-208: Addingfeeabstypes.ModuleNameto themaccPermsmap is correct, ensuring the fee abstraction module has the appropriate permissions.
616-627: The instantiation ofFeeabsKeeperis correctly implemented, with all necessary dependencies passed in. Ensure that theBankKeeperargument matches the expected interface.
658-665: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [629-662]
The setup of the fee abstraction IBC module and its integration into the IBC router is correctly done, ensuring that the fee abstraction functionality is available for IBC operations.
568-569: The integration of the fee abstraction module into the governance router, module manager, and genesis module order is correctly done, ensuring that the module is properly initialized and can participate in governance proposals.Also applies to: 790-790, 846-867, 914-914
Resolve #125
Summary by CodeRabbit